-
Notifications
You must be signed in to change notification settings - Fork 351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow APIOptions.trackInteraction()'s arg object to have extra keys #616
Conversation
🦋 Changeset detectedLatest commit: 0e920b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Change: -45 B (0%) Total Size: 823 kB
ℹ️ View Unchanged
|
type Rubric = PerseusSequenceWidgetOptions; | ||
|
||
type ExternalProps = WidgetProps<PerseusSequenceWidgetOptions, Rubric>; | ||
|
||
type Props = ExternalProps; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is our more modern way to define prop types for widgets. It uses WidgetProps<>
as that defines all the standard props that the Renderer
hands down to every widget. The PerseusSequenceWidgetOptions
are then the option data that the content author configured for this widget instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes here are just migrating this widget to use TypeScript types for Props and State.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome, thank you 🌈
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I wonder if this has to do with why I had some issues in the last deploy: https://github.com/Khan/webapp/pull/15270/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome, thank you 🌈
Yes, that was definitely part of the issues we saw in that release. |
Summary:
In #609 I clarified the trackInteraction API types. However, I missed defining that the
APIOptions.trackInteraction
argument object can have extra, arbitrary keys provided by the widget that originates the interaction.This PR adds that to the
trackInteraction
function type.Issue: "none"
Test plan:
yarn tsc
yarn test